-
-
Notifications
You must be signed in to change notification settings - Fork 271
[navigation menu] "Fix" Safari issue where positionerElement's width is set to 0 on hover #3309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[navigation menu] "Fix" Safari issue where positionerElement's width is set to 0 on hover #3309
Conversation
commit: |
Bundle size report
Check out the code infra dashboard for more information about this PR. |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
For some reason, I can't reproduce the original issue on Safari at all (26.1)
It ends up breaking the transitions of the size of the popup, so doesn't seem so.
It's possible Falling back may help: const handleValueChange = useStableCallback((currentWidth: number, currentHeight: number) => {
if (!popupElement || !positionerElement) {
return;
}
popupElement.style.removeProperty(NavigationMenuPopupCssVars.popupWidth);
popupElement.style.removeProperty(NavigationMenuPopupCssVars.popupHeight);
positionerElement.style.removeProperty(NavigationMenuPositionerCssVars.positionerWidth);
positionerElement.style.removeProperty(NavigationMenuPositionerCssVars.positionerHeight);
const { width, height } = getCssDimensions(popupElement);
const measuredWidth = width || prevSizeRef.current.width;
const measuredHeight = height || prevSizeRef.current.height;
if (currentHeight === 0 || currentWidth === 0) {
currentWidth = measuredWidth;
currentHeight = measuredHeight;
}
popupElement.style.setProperty(NavigationMenuPopupCssVars.popupWidth, `${currentWidth}px`);
popupElement.style.setProperty(NavigationMenuPopupCssVars.popupHeight, `${currentHeight}px`);
positionerElement.style.setProperty(
NavigationMenuPositionerCssVars.positionerWidth,
`${measuredWidth}px`,
);
positionerElement.style.setProperty(
NavigationMenuPositionerCssVars.positionerHeight,
`${measuredHeight}px`,
);
sizeFrame1.request(() => {
popupElement.style.setProperty(NavigationMenuPopupCssVars.popupWidth, `${measuredWidth}px`);
popupElement.style.setProperty(
NavigationMenuPopupCssVars.popupHeight,
`${measuredHeight}px`,
);
sizeFrame2.request(() => {
animationAbortControllerRef.current = new AbortController();
runOnceAnimationsFinish(setAutoSizes, animationAbortControllerRef.current.signal);
});
});
});Or figuring out why the self-correction via the layout effect doesn't occur in your case |
Sorry I forgot to add which version I'm experiencing the behavior at. I'm running the version before macOS 26. Seems to only be an issue on ~18.1.1 (20619.2.8.11.12) in that case!
Tried to apply the above, it'll not fallback to anything with a size unfortunately. In the scope the Though, and if it makes sense to change, it'll self-correct if it wouldn't reset the React.useEffect(() => {
if (!open) {
stickIfOpenTimeout.clear();
sizeFrame1.cancel();
sizeFrame2.cancel();
- prevSizeRef.current = DEFAULT_SIZE;
}
}, [stickIfOpenTimeout, open, sizeFrame1, sizeFrame2]);Screen.Recording.2025-11-24.at.09.21.52.movSeems like figuring out why |
|
Try changing |
That does work! Screen.Recording.2025-11-24.at.15.25.48.mov |
|
Alright, nice! The other side effects still need to be done with |
Possible fix for: #3308
Writing possible because:
Screen.Recording.2025-11-23.at.18.46.51.mov
getCssDimensionsis invoked beforesizeFrame1.request, it'll get into the buggy state again.Here's an recording that shows how it's behaving on master, then on this PR's change and lastly adding a
getCssDimensionscall beforesizeFrame1.requestexecutes:Screen.Recording.2025-11-23.at.18.51.22.mov
This is really surprised me. Something with invoking
getCssDimensionscauses it to behave differently.